Skip to content

fix(pool): close warm-pool shutdown-vs-replenish push race (orphaned-VM leak, MED) - #145

Merged
ZhiXiao-Lin merged 1 commit into
mainfrom
fix/warmpool-shutdown-push-race
Jun 17, 2026
Merged

fix(pool): close warm-pool shutdown-vs-replenish push race (orphaned-VM leak, MED)#145
ZhiXiao-Lin merged 1 commit into
mainfrom
fix/warmpool-shutdown-push-race

Conversation

@ZhiXiao-Lin

Copy link
Copy Markdown
Contributor

From the concurrency audit. A freshly-booted warm VM could be pushed into the idle pool after drain_idle already cleared it, leaking the VM (overlay mount + IP + volumes) — there is no Drop reaper.

Race

The replenish task checked *shutdown_rx.borrow() and only then acquired the idle lock to push — with an await in between:

if *shutdown_rx.borrow() { destroy; continue }   // unlocked check
let mut pool = idle.lock().await;                 // ← drain_idle can run here
pool.push(vm);                                    // pushed into a drained pool → leak

drain_all does signal_shutdown() then drain_idle() (locks idle, drains, runs once). If the unlocked check saw false and drain_idle completed before the task acquired the lock, the VM landed in the already-drained pool and was never reclaimed.

Fix

Acquire the idle lock first, then re-check shutdown under it before pushing — making check-and-push atomic against drain_idle (which holds the same lock while draining, always after signal_shutdown). On shutdown, destroy the VM instead of pushing. The same latent leak existed in release() (returns a VM to the pool with no shutdown check) — guarded the same way.

Validation

Full a3s-box-runtime pool/lib suite (55) green, fmt + clippy clean. A minimal check-under-lock change; the interleaving itself needs a real-VM harness to exercise (like the kill-identity / durable-write fixes).

Third of the 4 concurrency findings (after #144's HIGH monitor-resurrect + MED kill-identity). Remaining: concurrent restart+monitor-boot → orphan VM (HIGH).

…VM leak)

From the concurrency audit. A freshly-booted warm VM could be pushed into the
idle pool AFTER drain_idle already cleared it, leaking the VM (mount/IP/volume)
since there is no Drop reaper.

The replenish task checked `*shutdown_rx.borrow()` and only THEN acquired the
idle lock to push — with an await in between. drain_all does signal_shutdown()
then drain_idle() (which locks idle, drains, and runs exactly once). If the
unlocked check observed shutdown=false and drain_idle completed before the task
acquired the lock, the VM was pushed into the already-drained pool and never
reclaimed.

Fix: acquire the idle lock FIRST, then re-check shutdown UNDER it before pushing
— making the check-and-push atomic against drain_idle (which holds the same lock
while draining, always after signal_shutdown). On shutdown, destroy the VM
instead of pushing. The same latent leak existed in `release()` (returns a VM to
the pool with no shutdown check); guard it the same way.

Validated: full a3s-box-runtime pool/lib suite (55) green, fmt + clippy clean.
A minimal check-under-lock change; the interleaving itself needs a real-VM
harness to exercise (like the kill-identity / durable-write fixes).
@ZhiXiao-Lin
ZhiXiao-Lin merged commit 0e4ed61 into main Jun 17, 2026
8 checks passed
@ZhiXiao-Lin
ZhiXiao-Lin deleted the fix/warmpool-shutdown-push-race branch June 17, 2026 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant